[DOCS] Add code snippet testing in more ML APIs #31339

lcawl · 2018-06-14T17:19:37Z

This PR adds code snippet testing for more machine learning APIs.

Related to #30665

elasticmachine · 2018-06-14T17:19:39Z

Pinging @elastic/ml-core

elasticmachine · 2018-06-14T17:19:40Z

Pinging @elastic/es-docs

davidkyle · 2018-06-15T12:48:08Z

The build is failing for the preview-datafeed line 52 snippet with the error

ERROR   1.04s | XDocsClientYamlTestSuiteIT.test {yaml=en/rest-api/ml/preview-datafeed/line_52} <<< FAILURES!
   > Throwable #1: java.lang.RuntimeException: Failure at [en/rest-api/ml/preview-datafeed:75]: element was a list, but [_shards] was not numeric

The problem is that the test framework injects the assertion - is_false: _shards.failures into the yml test but the response is a JSON array rather than an object so the check for _shards.failures throws an exception because _shards can't be an index to the array.

This feels like a corner case -I haven't seen this problem before-, one solution is the change the format of the preview datafeed response to be NDJSON although technically that is a breaking change. Another option is to add a flag to the snippet expects_array that would forgo the _shards.failures check.

@nik9000 do you have any ideas please?

The generated yml test looks like this:

  - do:
      raw:
        method: GET
        path: "_xpack/ml/datafeeds/datafeed-farequote/_preview"
  - is_false: _shards.failures
  - match: 
      $body: 
        [
          {
            "time": 1454803200000,
            "airline": "JZA",
            "doc_count": 5,
            "responsetime": 990.4628295898438
          },
          {
            "time": 1454803200000,
            "airline": "JBU",
            "doc_count": 23,
            "responsetime": 877.5927124023438
          },
          {
            "time": 1454803200000,
            "airline": "KLM",
            "doc_count": 42,
            "responsetime": 1355.481201171875
          }
        ]

nik9000 · 2018-06-18T17:48:59Z

@davidkyle, we're talking about this API, right?

That assertion is really just there because searches are so command and we don't otherwise check for failing shards. I wonder if we'd be better off just asserting that after a search.

davidkyle · 2018-06-19T10:06:29Z

@nik9000 Yes the preview data feed API.

It would be nice to enable the check only for searches (and GET document/bulk APIs?) as it is redundant for the majority for cases especially in x-pack. The change appears a simple one I'll have a go at it if you don't mind.

I would also be happy to change the format of the preview API and make it a breaking change in 7.

nik9000 · 2018-06-19T13:24:30Z

I would also be happy to change the format of the preview API and make it a breaking change in 7.

I don't have a problem with the format of the preview API. I don't really know it well enough to have an opinion.

The change appears a simple one I'll have a go at it if you don't mind.

Sure! A whitelist of a few APIs we should generate this for is fine with me!

droberts195 · 2018-06-19T14:06:54Z

I would also be happy to change the format of the preview API and make it a breaking change in 7.

Before doing this we need to consider:

Are there any other APIs that return ND-JSON as a result type?
Do the client maintainers have to do bespoke work to consume ND-JSON as a result type for a particular API?

If the answers are no and yes then we shouldn't make this change. If the answers are yes and yes then we need to carefully weigh up the benefit against the extra work we're piling onto the clients team.

lcawl · 2018-06-19T17:17:23Z

(I am moving all but the preview datafeed API work out of this PR). The forecast API piece is moving to #31439

davidkyle · 2018-06-20T16:33:12Z

@lcawl I merged the fix for the datafeed preview API (#31430) into 7, 6.4 and 6.3.1 if you rebase you should get a green build now.

* master: Add get field mappings to High Level REST API Client (#31423) [DOCS] Updates Watcher examples for code testing (#31152) TEST: Add bwc recovery tests with synced-flush index [DOCS] Move sql to docs (#31474) [DOCS] Move monitoring to docs folder (#31477) Core: Combine doExecute methods in TransportAction (#31517) IndexShard should not return null stats (#31528) fix repository update with the same settings but different type (#31458) Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527) Node selector per client rather than per request (#31471) Core: Combine messageRecieved methods in TransportRequestHandler (#31519) Upgrade to Lucene 7.4.0. (#31529) [ML] Add ML filter update API (#31437) Allow multiple unicast host providers (#31509) Avoid deprecation warning when running the ML datafeed extractor. (#31463) REST high-level client: add simulate pipeline API (#31158) Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507) [PkiRealm] Invalidate cache on role mappings change (#31510) [Security] Check auth scheme case insensitively (#31490) In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514) [DOCS] Fix REST tests in SQL docs [DOCS] Add code snippet testing in more ML APIs (#31339) Core: Remove ThreadPool from base TransportAction (#31492) [DOCS] Remove fixed file from build.gradle Rename createNewTranslog to fileBasedRecovery (#31508) Test: Skip assertion on windows [DOCS] Creates field and document level security overview (#30937) [DOCS] Significantly improve SQL docs [DOCS] Move migration APIs to docs (#31473) Core: Convert TransportAction.execute uses to client calls (#31487) Return transport addresses from UnicastHostsProvider (#31426) Ensure local addresses aren't null (#31440) Remove unused generic type for client execute method (#31444) Introduce http and tcp server channels (#31446)

* 6.x: Avoid sending duplicate remote failed shard requests (#31313) Add get field mappings to High Level REST API Client Relates to #27205 [DOCS] Updates Watcher examples for code testing (#31152) [DOCS] Move monitoring to docs folder (#31477) [DOCS] Fixes SQL docs in nav [DOCS] Move sql to docs IndexShard should not return null stats - empty stats or AlreadyCloseException if it's closed is better Clarify that IP range data can be specified in CIDR notation. (#31374) Remove some cases in FieldTypeLookupTests that are no longer relevant. (#31381) In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514) fix repository update with the same settings but different type Revert "AwaitsFix FullClusterRestartIT#testRecovery" Upgrade to Lucene 7.4.0. (#31529) Avoid deprecation warning when running the ML datafeed extractor. (#31463) Retry synced-flush in FullClusterRestartIT#testRecovery Allow multiple unicast host providers (#31509) [ML] Add ML filter update API (#31437) AwaitsFix FullClusterRestartIT#testRecovery Fix missing historyUUID in peer recovery when rolling upgrade 5.x to 6.3 (#31506) Remove QueryCachingPolicy#ALWAYS_CACHE (#31451) Rename createNewTranslog to fileBasedRecovery (#31508) [DOCS] Add code snippet testing in more ML APIs (#31339) [DOCS] Remove fixed file from build.gradle [DOCS] Creates field and document level security overview (#30937) Test: Skip assertion on windows [DOCS] Move migration APIs to docs (#31473) Add a known issue for upgrading from 5.x to 6.3.0 (#31501) Return transport addresses from UnicastHostsProvider (#31426) Add Delete Snapshot High Level REST API Reload secure settings for plugins (#31481) [DOCS] Fix JDBC Maven client group/artifact ID

lcawl added v7.0.0 :ml Machine learning v6.4.0 v6.3.1 :Docs labels Jun 14, 2018

lcawl added the WIP label Jun 14, 2018

davidkyle mentioned this pull request Jun 19, 2018

[DOCS] Omit shard failures assertion for incompatible responses #31430

Merged

lcawl force-pushed the lcawley-ml-tests branch from c835118 to 31aad9e Compare June 19, 2018 16:21

lcawl force-pushed the lcawley-ml-tests branch from 31aad9e to a6e9355 Compare June 20, 2018 16:54

lcawl removed the WIP label Jun 20, 2018

lcawl added 4 commits June 20, 2018 15:29

[DOCS] Test validate job API

9068d45

[DOCS] Test ML foreast API

32394ce

[DOCS] Adds test for ML preview API

0353a8c

[DOCS] Fixes integer in build.gradle

a713e01

lcawl force-pushed the lcawley-ml-tests branch from a6e9355 to a713e01 Compare June 20, 2018 22:29

lcawl merged commit 4385915 into elastic:master Jun 21, 2018

lcawl deleted the lcawley-ml-tests branch June 21, 2018 18:32

lcawl added a commit that referenced this pull request Jun 21, 2018

[DOCS] Add code snippet testing in more ML APIs (#31339)

e0bc4d5

lcawl added a commit to lcawl/elasticsearch that referenced this pull request Jun 21, 2018

[DOCS] Add code snippet testing in more ML APIs (elastic#31339)

71d52d9

tomcallahan added >docs General docs changes and removed :Docs labels Jul 5, 2018

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DOCS] Add code snippet testing in more ML APIs #31339

[DOCS] Add code snippet testing in more ML APIs #31339

lcawl commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

davidkyle commented Jun 15, 2018

nik9000 commented Jun 18, 2018

davidkyle commented Jun 19, 2018

nik9000 commented Jun 19, 2018

droberts195 commented Jun 19, 2018

lcawl commented Jun 19, 2018

davidkyle commented Jun 20, 2018

[DOCS] Add code snippet testing in more ML APIs #31339

[DOCS] Add code snippet testing in more ML APIs #31339

Conversation

lcawl commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

elasticmachine commented Jun 14, 2018

davidkyle commented Jun 15, 2018

nik9000 commented Jun 18, 2018

davidkyle commented Jun 19, 2018

nik9000 commented Jun 19, 2018

droberts195 commented Jun 19, 2018

lcawl commented Jun 19, 2018

davidkyle commented Jun 20, 2018